Repository navigation
Conversation
There was a problem hiding this comment.
40 issues found across 16 files
Confidence score: 1/5
- The highest risk is widespread compile-time breakage in
scripts/MissionHelper/MissionHelper.gmlandscripts/scr_mission_functions/scr_mission_functions.gml: malformed switches, invalid:syntax, duplicate cases, and an unclosed function can prevent the scripts from parsing—restore the syntax and verify the project builds. - Runtime problem handling is also likely to fail in
scripts/scr_enemy_ai_d/scr_enemy_ai_d.gmlandscripts/scr_PlanetData/scr_PlanetData.gmlbecause code references undefined or wrong variables and deletes array elements at index0; correct the names and deletion index before exercising turn-end and expiry flows. MissionHelperconstructors leavetimer,data, andplanetunset due to parameter shadowing, whilescripts/scr_inquisition_mission/scr_inquisition_mission.gmlpasses the wrong estimate field; initialize the instance fields explicitly and usepop_data.estimateto avoid broken countdowns and prematurely due missions.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/scr_enemy_ai_d/scr_enemy_ai_d.gml">
<violation number="1" location="scripts/scr_enemy_ai_d/scr_enemy_ai_d.gml:84">
P0: `p_problems` is never defined anywhere in the codebase (the actual parallel array is `p_problem`, initialized in objects/obj_star/Create_0.gml and populated via PlanetData.register_problem). Reading the undefined instance variable `p_problems` throws a runtime error on this line on every star step, so the entire problem-processing loop is dead before it runs. This looks like a typo for `p_problem[i]`.</violation>
<violation number="2" location="scripts/scr_enemy_ai_d/scr_enemy_ai_d.gml:87">
P1: `problem_count_down` was removed from scr_mission_functions (where it was a global, obj_star-scoped function) and is now a method bound to the PlanetData struct (scripts/scr_PlanetData/scr_PlanetData.gml:790, `function problem_count_down(...)` inside the constructor). scr_enemy_ai_d() runs with self = obj_star, which no longer has this method, so this call raises an unknown-function/variable error. Call it on the PlanetData instance instead (it is already fetched as `_pdata` on the next line).</violation>
<violation number="3" location="scripts/scr_enemy_ai_d/scr_enemy_ai_d.gml:95">
P1: Custom agent: **Code Quality Review**
The removed end-turn path has no working equivalent: `PlanetProblem.basic_turn_end()` does not normally decrement timers and dispatches `hunt_beast` to the undefined `resolve_hunt_beast`. Preserve the legacy countdown, storm adjustment, and completion mappings in the new mission object.</violation>
</file>
<file name="scripts/scr_mission_functions/scr_mission_functions.gml">
<violation number="1" location="scripts/scr_mission_functions/scr_mission_functions.gml:91">
P0: Custom agent: **Code Quality Review**
Deleting the closing brace leaves `problem_end_turn_checks` unclosed, so the script is malformed and the following function declarations are not reliably parsed. Restore the closing brace after the function, or remove the obsolete declaration entirely.</violation>
</file>
<file name="scripts/MissionHelper/MissionHelper.gml">
<violation number="1" location="scripts/MissionHelper/MissionHelper.gml:8">
P0: The constructor shadows its instance fields, so `timer = timer` and `data = data` assign the arguments to themselves. Every problem then lacks the fields used by countdown and mission callbacks; rename the parameters or assign through `self`.</violation>
<violation number="2" location="scripts/MissionHelper/MissionHelper.gml:8">
P0: The constructor never initializes the `timer`, `data`, and `planet` struct members because those names collide with the constructor parameters. In GML, constructor arguments are local variables, so `timer = timer;`, `data = data;`, and `planet = planet;` each assign the local to itself and never create a member on the struct (the docs require `self.timer = timer;` for same-name members). Every static method that reads `timer`, `data`, or `planet` (`basic_turn_end`, `refresh_p_data`, `increment_mission_completion`, `per_turn_check_mech_*`, etc.) then reads an unset member, so the whole PlanetProblem object is broken. Use `self.` on these three assignments.</violation>
<violation number="3" location="scripts/MissionHelper/MissionHelper.gml:8">
P1: Custom agent: **Code Quality Review**
The constructor never stores the mission timer or data on the generated struct. Because the arguments shadow the intended instance fields, both assignments are self-assignment no-ops; use `self.timer = timer` and `self.data = data` (or rename the arguments) so `basic_turn_end` and completion tracking can access the state.</violation>
<violation number="4" location="scripts/MissionHelper/MissionHelper.gml:16">
P2: Custom agent: **Code Quality Review**
This new GML file uses tab indentation in many blocks, violating CODE_STYLE.md's requirement to use 4 spaces and avoid tabs. Replace leading tabs throughout the file with four spaces.</violation>
<violation number="5" location="scripts/MissionHelper/MissionHelper.gml:35">
P0: The per-turn dispatch switch (lines 34-50) is syntactically invalid and will not compile, breaking the whole build. `case "mech_raider";` and `case "awaiting_player";` use semicolons instead of colons, `switch(stage_id):` (line 43) uses a colon instead of braces with a stray `break;` immediately after, and `case "mech_bionics":` is duplicated (lines 39 and 42). Separately, line 36 assigns `_func = per_turn_check_raider_failed;`, but that function is never defined anywhere in the repo (only `per_turn_check_mech_raider` exists at line 470), so the mech_raider per-turn check would silently never run even if the syntax were fixed. Rewrite the switch with proper `case X:` labels and a single `mech_bionics` branch dispatching on `stage_id`, mapping `mech_raider` to `per_turn_check_mech_raider`.</violation>
<violation number="6" location="scripts/MissionHelper/MissionHelper.gml:36">
P1: The raider per-turn case references an undefined handler, so raider progress cannot run. Assign `per_turn_check_mech_raider` instead.</violation>
<violation number="7" location="scripts/MissionHelper/MissionHelper.gml:42">
P0: Custom agent: **Code Quality Review**
The added `basic_turn_end` switch cannot compile: it duplicates `case "mech_bionics"` and writes the nested switch as `switch(stage_id):` instead of `switch (stage_id) {`. Replace the nested switch with valid brace syntax, remove the duplicate outer case, and place `break;` only within a case.</violation>
<violation number="8" location="scripts/MissionHelper/MissionHelper.gml:61">
P1: This zero-timer branch invokes one-shot resolvers without retiring the problem. Penalties, popups, and rewards repeat every turn while `timer == 0`; disable the checks or remove the problem after each resolver.</violation>
<violation number="9" location="scripts/MissionHelper/MissionHelper.gml:65">
P1: The `hunt_beast` expiry case references an undefined callback, so the mission cannot complete at expiry. Dispatch to `complete_beast_hunt_mission`.</violation>
<violation number="10" location="scripts/MissionHelper/MissionHelper.gml:158">
P1: This call uses `man_conditions` before its declaration and initialization. Move the conditions struct before `collect_role_group`, otherwise beast-hunt completion receives undefined filters.</violation>
<violation number="11" location="scripts/MissionHelper/MissionHelper.gml:325">
P1: When no trainer remains on the planet, `_trainer` is still an array but this line accesses `.job`. Guard the cleanup with a unit check.</violation>
<violation number="12" location="scripts/MissionHelper/MissionHelper.gml:479">
P1: When the raider mission completes, this passes `id` instead of the stored star object. `scr_mission_reward` reads `star.name`, so completion fails; pass `system` or the correct star instance.</violation>
<violation number="13" location="scripts/MissionHelper/MissionHelper.gml:599">
P0: Custom agent: **Code Quality Review**
This added statement uses `:` outside a struct literal, so the GML script cannot compile. Assign the mission counter with `=` before `per_turn_check_mech_tomb2` increments it.</violation>
<violation number="14" location="scripts/MissionHelper/MissionHelper.gml:599">
P1: `data.turns : 0;` uses a colon instead of an assignment operator and is a syntax error that will break compilation. Replace it with `data.turns = 0;` (the intent is to reset the tomb-research turn counter when the exploring stage begins).</violation>
</file>
<file name="scripts/scr_PlanetData/scr_PlanetData.gml">
<violation number="1" location="scripts/scr_PlanetData/scr_PlanetData.gml:777">
P1: `new_problem` changes collection entries from string IDs to `PlanetProblem` structs, but this file’s lookup and removal methods still use helpers that compare entries directly with strings. After registration is fixed, mission checks and resolution cleanup will not find these objects; make those helpers compare `p_id` or use object-aware operations.</violation>
<violation number="2" location="scripts/scr_PlanetData/scr_PlanetData.gml:785">
P1: Every default `new_problem` call enters `register_problem`, but `p_problem` is not a field of this `PlanetData` struct. Push into `system.p_problem[planet]` so succession and fallen problems are actually registered.</violation>
<violation number="3" location="scripts/scr_PlanetData/scr_PlanetData.gml:785">
P1: Custom agent: **Code Quality Review**
When `new_problem()` uses its default `register = true`, `register_problem()` evaluates `p_problem`, but `PlanetData` never initializes that variable. This can fail before registering the new problem; push to `system.p_problem[planet]` (or the correctly scoped `problems` array) instead.</violation>
<violation number="4" location="scripts/scr_PlanetData/scr_PlanetData.gml:785">
P1: `register_problem` pushes to a bare `p_problem` that is not an instance variable of PlanetData (grep shows it only at lines 785-786 in this file), so it throws. It should push into `system.p_problem[planet]`. Additionally, because obj_star now initializes `p_problem` as a flat `[]`, `system.p_problem[planet]` needs per-planet array initialization (`array_create_advanced(_planet_array_size, [])`) or `array_length(problems)` in `problem_count_down` will operate on an undefined/non-array value.</violation>
<violation number="5" location="scripts/scr_PlanetData/scr_PlanetData.gml:794">
P0: Custom agent: **Code Quality Review**
When a problem expires, this function checks the undeclared `problem` instead of `_problem`, then deletes from `system.p_problem[i]` and requests deletion of zero elements. The turn loop calls this for every planet, so expired `PlanetProblem` entries either trigger an undefined-variable error or remain registered. Use `_problem.timer`, `system.p_problem[planet]`, and a deletion count of `1`.</violation>
<violation number="6" location="scripts/scr_PlanetData/scr_PlanetData.gml:794">
P1: The loop item is `_problem`, but this condition reads the undeclared `problem` value. On countdown, use `_problem.timer` or expired problems will error or fail to be removed.</violation>
<violation number="7" location="scripts/scr_PlanetData/scr_PlanetData.gml:794">
P0: `problem_count_down` reads `problem.timer`, but the loop variable is `_problem`; `problem` is undefined in this scope and throws at runtime whenever a planet has a registered problem. Also both `array_delete(..., 0)` calls pass amount 0, so expired problems are never actually removed. Use the `_problem` variable and the `remove` flag that `basic_turn_end()` maintains, deleting 1 element.</violation>
<violation number="8" location="scripts/scr_PlanetData/scr_PlanetData.gml:795">
P1: After the timer check is fixed, this cleanup still indexes `system.p_problem` with the problem slot and requests deletion of zero entries. Delete one entry from the current planet collection once.</violation>
</file>
<file name="scripts/scr_inquisition_mission/scr_inquisition_mission.gml">
<violation number="1" location="scripts/scr_inquisition_mission/scr_inquisition_mission.gml:229">
P1: When the saved target system no longer exists, this line calls `get_planet_data` on `noone` before the existing guard can close the popup. Move the planet lookup below the `mission_star == noone` check.</violation>
<violation number="2" location="scripts/scr_inquisition_mission/scr_inquisition_mission.gml:251">
P1: For these structured popups, `estimate` is the popup field initialized to `0`, not `pop_data.estimate`. Passing it registers the Necron problem as due immediately; pass `pop_data.estimate` so the mission uses its advertised ETA.</violation>
</file>
<file name="objects/obj_popup/Step_0.gml">
<violation number="1" location="objects/obj_popup/Step_0.gml:215">
P1: `new_problem` exists only as a static method on PlanetData; there is no global `new_problem` function, so this bare call crashes and the recon mission is never registered. Call it on the planet-data struct: `_pdata.new_problem("recon", estimate)`. The assigned `_problem` is unused and can be dropped.</violation>
<violation number="2" location="objects/obj_popup/Step_0.gml:237">
P1: For every valid Inquisition planet mission, this call enters `PlanetData.register_problem`, which currently appends to the nonexistent `p_problem` field and throws. Fix the registration method to append the object to the target planet's problem collection before relying on this call; otherwise missions are never accepted.</violation>
</file>
<file name="scripts/scr_mechanicus_missions/scr_mechanicus_missions.gml">
<violation number="1" location="scripts/scr_mechanicus_missions/scr_mechanicus_missions.gml:146">
P1: A newly accepted tomb mission never enters `per_turn_check_mech_tomb1` because the OOP dispatcher has no `mech_tomb` case. It therefore expires into the failure handler after 17 months instead of waiting for the required Astartes and starting the research; add the tomb dispatch case before using this new problem ID.</violation>
<violation number="2" location="scripts/scr_mechanicus_missions/scr_mechanicus_missions.gml:146">
P1: Every mission acceptance path now fails while registering the new `PlanetProblem`: `register_problem` pushes to an undefined `p_problem` instead of this planet's problem array. Register the object in `system.p_problem[planet]` (or the `problems` alias) before switching these callers to `new_problem`.</violation>
<violation number="3" location="scripts/scr_mechanicus_missions/scr_mechanicus_missions.gml:171">
P1: After registration is fixed, these object-backed missions still cannot advance because `problem_count_down` dereferences the undefined `problem` name after running `_problem.basic_turn_end()`. Fix the shared loop to use the same `_problem` object before enabling these new callers.</violation>
</file>
<file name="scripts/scr_destroy_planet/scr_destroy_planet.gml">
<violation number="1" location="scripts/scr_destroy_planet/scr_destroy_planet.gml:167">
P1: Custom agent: **Code Quality Review**
When a planet is created or destroyed, `p_timer` and `p_problem_other_data` are no longer initialized, but mission and planet-data code still indexes both arrays. Restore their initialization (or migrate every remaining access) before resetting `p_problem`, otherwise those paths can read or write undefined arrays.</violation>
</file>
<file name="scripts/scr_crusade/scr_crusade.gml">
<violation number="1" location="scripts/scr_crusade/scr_crusade.gml:236">
P1: This invokes the default registration path, which references undefined `p_problem` in `PlanetData.register_problem`, so the crusade cannot be registered. Fix `register_problem` to append to `system.p_problem[planet]` before using this call.</violation>
<violation number="2" location="scripts/scr_crusade/scr_crusade.gml:236">
P1: This passes the star instance as `PlanetProblem.data`, but the new API expects a struct payload and immediately inspects it as one. Omit the third argument; `_p_data` already identifies the crusade location.</violation>
</file>
<file name="scripts/scr_chaos_alliance_test/scr_chaos_alliance_test.gml">
<violation number="1" location="scripts/scr_chaos_alliance_test/scr_chaos_alliance_test.gml:98">
P1: When the alliance roll produces `success_trap`, this call creates `meeting_trap` but no longer emits the response, event log, or purple marker. Handle `meeting_trap` in the problem initialization or restore the shared side effects for both outcomes.</violation>
</file>
<file name="objects/obj_star/Create_0.gml">
<violation number="1" location="objects/obj_star/Create_0.gml:70">
P1: Removing `p_problem_other_data` and `p_timer` from obj_star while scr_PlanetData's `refresh_data()` (lines 75-76) and the mission-display code (line 1429) still read `system.p_problem_other_data` / `system.p_timer` will throw on every planet refresh. The refactor is incomplete: scr_mission_functions.gml, scr_inquisition_mission.gml, scr_random_event.gml, and scr_enemy_ai_d.gml still reference the removed arrays and the old 2D layout, so the declarations should not be dropped until all consumers are migrated.</violation>
</file>
<file name="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml">
<violation number="1" location="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml:268">
P1: `mission_name_key(problems[p])` now receives a PlanetProblem object instead of the mission key string, so `struct_exists(mission_key, object)` never matches and it always returns "none". Every mission is silently filtered out and the mission log renders empty. Pass the object's key instead: `mission_name_key(_problem.p_id)`.</violation>
<violation number="2" location="scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml:269">
P2: When `p_problem` contains an empty slot or a legacy string, `_problem.stage_id` throws before the mission log is rebuilt. Retain a type/empty guard here or migrate every writer before dereferencing OOP fields.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "succession": function(problem_index) { | ||
| if (problem_timers[problem_index] > 0) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
P0: Custom agent: Code Quality Review
Deleting the closing brace leaves problem_end_turn_checks unclosed, so the script is malformed and the following function declarations are not reliably parsed. Restore the closing brace after the function, or remove the obsolete declaration entirely.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_mission_functions/scr_mission_functions.gml, line 91:
<comment>Deleting the closing brace leaves `problem_end_turn_checks` unclosed, so the script is malformed and the following function declarations are not reliably parsed. Restore the closing brace after the function, or remove the obsolete declaration entirely.</comment>
<file context>
@@ -17,7 +17,7 @@ global.planet_problem_keys = [
"mech_raider",
"mech_bionics",
"mech_mars",
- "mech_tomb1",
+ "mech_tomb",
"fallen",
"great_crusade",
"harlequins",
@@ -42,14 +42,14 @@ global.planet_problem_keys = [
</file context>
| _func = per_turn_check_mech_bionics; | ||
| break; | ||
| case "mech_bionics": | ||
| switch(stage_id): |
There was a problem hiding this comment.
P0: Custom agent: Code Quality Review
The added basic_turn_end switch cannot compile: it duplicates case "mech_bionics" and writes the nested switch as switch(stage_id): instead of switch (stage_id) {. Replace the nested switch with valid brace syntax, remove the duplicate outer case, and place break; only within a case.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/MissionHelper/MissionHelper.gml, line 42:
<comment>The added `basic_turn_end` switch cannot compile: it duplicates `case "mech_bionics"` and writes the nested switch as `switch(stage_id):` instead of `switch (stage_id) {`. Replace the nested switch with valid brace syntax, remove the duplicate outer case, and place `break;` only within a case.</comment>
<file context>
@@ -0,0 +1,617 @@
+ _func = per_turn_check_mech_bionics;
+ break;
+ case "mech_bionics":
+ switch(stage_id):
+ break;
+ case "exploring":
</file context>
| if (array_length(_marines) >= 20) { | ||
| stage_id = "exploring"; | ||
| timer = 999; | ||
| data.turns : 0; |
There was a problem hiding this comment.
P0: Custom agent: Code Quality Review
This added statement uses : outside a struct literal, so the GML script cannot compile. Assign the mission counter with = before per_turn_check_mech_tomb2 increments it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/MissionHelper/MissionHelper.gml, line 599:
<comment>This added statement uses `:` outside a struct literal, so the GML script cannot compile. Assign the mission counter with `=` before `per_turn_check_mech_tomb2` increments it.</comment>
<file context>
@@ -0,0 +1,617 @@
+ if (array_length(_marines) >= 20) {
+ stage_id = "exploring";
+ timer = 999;
+ data.turns : 0;
+ scr_popup("Mechanicus Research", $"The Mechanicus Research team on planet {p_data.name()} has taken note of your Astartes and are now prepared to begin their research. Your marines are to stay on the planet until further notice.", "necron_cave", "");
+ }
</file context>
| data.turns : 0; | |
| data.turns = 0; |
| function SystemProblem(name, timer, data){} | ||
|
|
||
| function PlanetProblem(name, timer, data, planet) constructor{ | ||
| timer = timer; |
There was a problem hiding this comment.
P0: The constructor shadows its instance fields, so timer = timer and data = data assign the arguments to themselves. Every problem then lacks the fields used by countdown and mission callbacks; rename the parameters or assign through self.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/MissionHelper/MissionHelper.gml, line 8:
<comment>The constructor shadows its instance fields, so `timer = timer` and `data = data` assign the arguments to themselves. Every problem then lacks the fields used by countdown and mission callbacks; rename the parameters or assign through `self`.</comment>
<file context>
@@ -0,0 +1,617 @@
+function SystemProblem(name, timer, data){}
+
+function PlanetProblem(name, timer, data, planet) constructor{
+timer = timer;
+uid = scr_uuid_generate();
+p_id = name;
</file context>
|
|
||
| static complete_beast_hunt_mission = function() { | ||
| refresh_p_data(); | ||
| var _hunters = collect_role_group("all", [system.name, planet, 0], false, man_conditions); |
There was a problem hiding this comment.
P1: This call uses man_conditions before its declaration and initialization. Move the conditions struct before collect_role_group, otherwise beast-hunt completion receives undefined filters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/MissionHelper/MissionHelper.gml, line 158:
<comment>This call uses `man_conditions` before its declaration and initialization. Move the conditions struct before `collect_role_group`, otherwise beast-hunt completion receives undefined filters.</comment>
<file context>
@@ -0,0 +1,617 @@
+
+static complete_beast_hunt_mission = function() {
+ refresh_p_data();
+ var _hunters = collect_role_group("all", [system.name, planet, 0], false, man_conditions);
+ if (stage_id == "active") {
+ var _mission_string = "";
</file context>
| } | ||
|
|
||
| static register_problem = function(problem){ | ||
| array_push(p_problem, problem); |
There was a problem hiding this comment.
P1: register_problem pushes to a bare p_problem that is not an instance variable of PlanetData (grep shows it only at lines 785-786 in this file), so it throws. It should push into system.p_problem[planet]. Additionally, because obj_star now initializes p_problem as a flat [], system.p_problem[planet] needs per-planet array initialization (array_create_advanced(_planet_array_size, [])) or array_length(problems) in problem_count_down will operate on an undefined/non-array value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_PlanetData/scr_PlanetData.gml, line 785:
<comment>`register_problem` pushes to a bare `p_problem` that is not an instance variable of PlanetData (grep shows it only at lines 785-786 in this file), so it throws. It should push into `system.p_problem[planet]`. Additionally, because obj_star now initializes `p_problem` as a flat `[]`, `system.p_problem[planet]` needs per-planet array initialization (`array_create_advanced(_planet_array_size, [])`) or `array_length(problems)` in `problem_count_down` will operate on an undefined/non-array value.</comment>
<file context>
@@ -775,9 +773,30 @@ function PlanetData(_planet, _system) constructor {
+ }
+
+ static register_problem = function(problem){
+ array_push(p_problem, problem);
+ problems = system.p_problem[planet];
+ }
</file context>
| @@ -0,0 +1,617 @@ | |||
| function MissionHelper() constructor{ | |||
There was a problem hiding this comment.
P2: Custom agent: Code Quality Review
This new GML file uses tab indentation in many blocks, violating CODE_STYLE.md's requirement to use 4 spaces and avoid tabs. Replace leading tabs throughout the file with four spaces.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/MissionHelper/MissionHelper.gml, line 16:
<comment>This new GML file uses tab indentation in many blocks, violating CODE_STYLE.md's requirement to use 4 spaces and avoid tabs. Replace leading tabs throughout the file with four spaces.</comment>
<file context>
@@ -0,0 +1,617 @@
+planet = p_data.planet;
+system = p_data.system;
+static refresh_p_data = function(){
+ p_data = system.get_planet_data(planet);
+}
+stage_id = "";
</file context>
| } | ||
| if (problem_has_key_and_value(i, p, "stage", "preliminary")) { | ||
| var _problem = problems[p]; | ||
| if (_problem.stage_id == "preliminary") { |
There was a problem hiding this comment.
P2: When p_problem contains an empty slot or a legacy string, _problem.stage_id throws before the mission log is rebuilt. Retain a type/empty guard here or migrate every writer before dereferencing OOP fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_unit_quick_find_pane/scr_unit_quick_find_pane.gml, line 269:
<comment>When `p_problem` contains an empty slot or a legacy string, `_problem.stage_id` throws before the mission log is rebuilt. Retain a type/empty guard here or migrate every writer before dereferencing OOP fields.</comment>
<file context>
@@ -265,20 +265,19 @@ function UnitQuickFindPanel() constructor {
- }
- if (problem_has_key_and_value(i, p, "stage", "preliminary")) {
+ var _problem = problems[p];
+ if (_problem.stage_id == "preliminary") {
continue;
}
</file context>
| if (_problem.stage_id == "preliminary") { | |
| if (!is_struct(_problem) || _problem.stage_id == "preliminary") { |
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Migrates mission initialization and completion logic into MissionHelper as methods, encapsulating mission state and behavior directly within mission objects. This change removes the reliance on parallel data arrays within PlanetData and scr_mission_functions, improving modularity and simplifying mission management by allowing direct method calls on mission instances.
Removes the global `add_new_problem` function, delegating problem addition to `PlanetData` objects via a new `add_problem` method. This change further encapsulates planet-specific state and behavior, reducing reliance on global functions and parallel data arrays. Updates `scr_inquisition_mission`, `scr_new_governor_mission`, and `scr_random_event` (Harlequins) to use the new `PlanetData` method. Additionally, centralizes Harlequin event popup and star marker logic within `MissionHelper`.
Enhances the encapsulation of planet problem logic by implementing direct management of problem objects within PlanetData methods. Removes obsolete global helper functions (`find_problem_planet`, `remove_planet_problem`, `open_problem_slot`, etc.) and updates call sites to utilize the PlanetData object's dedicated methods. This change further solidifies the object-oriented approach for handling planet problems, moving away from parallel arrays and external helper functions.
Introduces `new_end_turn_battle` in `MissionHelper` to abstract and centralize battle queuing. This new helper enables passing richer, structured data (via `special_feature`) to combat encounters. The `battle_mission` variable is deprecated and consolidated into `battle_special` to streamline battle-related context. Migrates per-turn battle logic from `scr_enemy_ai_e` into `MissionHelper`, enhancing modularity and aligning with the ongoing refactoring towards object-oriented design and better data encapsulation.
Moves pre-battle squad selection, in-battle special effects, and post-battle resolution logic from global scripts and `obj_ncombat` alarms into dedicated methods on `PlanetProblem` instances within `MissionHelper`. This significantly improves encapsulation by associating battle-related mission behavior directly with the `PlanetProblem` object (via `special_feature`), rather than relying on `battle_special` string parsing or scattered global functions. It also simplifies `obj_ncombat` by delegating special mission aftermath handling.
Moves hardcoded enemy definitions from `obj_ncombat` to a structured `battle_enemy_data` object, defined in `MissionHelper`. This allows mission-specific enemy forces to be defined declaratively, increasing flexibility and reducing reliance on conditional logic within the combat system. Introduces the `add_enemies` function in `obj_enunit` to dynamically populate units from this structured data. This continues the refactoring towards a more data-driven and encapsulated battle system.
Moves specific mission-based enemy definitions and combat flow logic from `obj_ncombat` and global scripts into `MissionHelper`. This includes the Necron Tomb, Fallen, and Spyrer encounters. Combat encounters now receive detailed enemy setups via the `battle_enemy_data` object, and pre/post-battle narratives are driven by `special_feature.data` flags. This greatly reduces reliance on `battle_special` string parsing, improving encapsulation and enabling a more flexible, data-driven approach to mission design.
There was a problem hiding this comment.
10 issues found across 28 files
Confidence score: 1/5
scripts/MissionHelper/MissionHelper.gmlcontainsdata.turns : 0;, which is invalid GML and prevents the script from compiling. Change:to=.- The planet-problem migration leaves live reads of
p_timerandp_problem_other_datawithout initialization, andproblem_count_downhas broken timer and deletion logic. Preserve or migrate those fields and fix the countdown path inscripts/scr_PlanetData/scr_PlanetData.gml. - Enemy encounters can fail to progress:
scr_enemy_ai_e.gmlno longer dispatches fallen checks or Necron tomb-raid setup, whileobj_enunit/Create_0.gmlstarts adding enemies at slot 1001 even thoughAlarm_1only processes slots 1–700. Restore the missing dispatches and use a valid enemy slot. MissionHelper.gmlpoints Mechanicus raider and beast-hunt missions at nonexistent callbacks, and Necron excursions pass a roster to a helper that does not accept it. Bind the real callbacks and align the battle helper’s roster interface.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="objects/obj_star/Create_0.gml">
<violation number="1" location="objects/obj_star/Create_0.gml:70">
P1: Custom agent: **Code Quality Review**
`p_timer` and `p_problem_other_data` are removed here while live code still reads and writes them, causing undefined-variable failures during enemy turns and inquisitor mission resolution. Migrate those consumers to the new mission objects and add a legacy save migration before removing these fields.</violation>
</file>
<file name="scripts/scr_enemy_ai_e/scr_enemy_ai_e.gml">
<violation number="1" location="scripts/scr_enemy_ai_e/scr_enemy_ai_e.gml:531">
P1: Custom agent: **Code Quality Review**
The deleted enemy-turn path is not fully migrated: `per_turn_check_fallen` is never dispatched, and no replacement calls `setup_necron_tomb_raid` for `necron` missions. Add the missing `PlanetProblem` turn-end integrations so Fallen battles and Necron tomb raids still trigger.</violation>
</file>
<file name="scripts/scr_destroy_planet/scr_destroy_planet.gml">
<violation number="1" location="scripts/scr_destroy_planet/scr_destroy_planet.gml:167">
P1: Custom agent: **Code Quality Review**
Legacy mission consumers still read `p_timer` and `p_problem_other_data`, but this path no longer initializes either array. Migrate those consumers to `PlanetProblem` or preserve the required state before destroying the planet.</violation>
</file>
<file name="scripts/scr_manage_task_selector/scr_manage_task_selector.gml">
<violation number="1" location="scripts/scr_manage_task_selector/scr_manage_task_selector.gml:51">
P1: Squad selection receives a `FeatureSelected` wrapper, not a `PlanetProblem`, so this type check rejects it and skips `on_squad_selection()`. Dereference the wrapped feature before testing it.</violation>
</file>
<file name="objects/obj_enunit/Create_0.gml">
<violation number="1" location="objects/obj_enunit/Create_0.gml:80">
P1: `add_enemies` selects index 1001 on a fresh unit, while `Alarm_1` only initializes indices 1–700, so data-driven battles contain no processed enemies. Find the first empty slot and stop the scan there.</violation>
</file>
<file name="scripts/scr_inquisition_mission/scr_inquisition_mission.gml">
<violation number="1" location="scripts/scr_inquisition_mission/scr_inquisition_mission.gml:227">
P1: This dereferences `mission_star` before the missing-star guard, so an invalid or deleted target crashes mission acceptance instead of closing the popup. Move `get_planet_data` after the guard.</violation>
</file>
<file name="scripts/MissionHelper/MissionHelper.gml">
<violation number="1" location="scripts/MissionHelper/MissionHelper.gml:37">
P1: These dispatch entries point to nonexistent callbacks, so Mechanicus raider progress and beast-hunt completion cannot execute. Bind each mission ID to the implementation that actually exists.</violation>
<violation number="2" location="scripts/MissionHelper/MissionHelper.gml:836">
P0: Custom agent: **Code Quality Review**
`data.turns : 0;` is invalid GML assignment syntax and prevents this script from compiling. Change `:` to `=`.</violation>
<violation number="3" location="scripts/MissionHelper/MissionHelper.gml:1274">
P1: Necron excursion battles pass their enemy roster to a parameter the helper does not accept or store, so the encounter either errors on argument count or starts without its generated enemies. Create the battle with two arguments and assign `_battle.battle_enemy_data = _battle_data`.</violation>
</file>
<file name="scripts/scr_PlanetData/scr_PlanetData.gml">
<violation number="1" location="scripts/scr_PlanetData/scr_PlanetData.gml:802">
P1: `problem_count_down` is broken: `problem.timer` is an undefined variable (should be `_problem.timer`), `array_delete(system.p_problem[i], i, 0)` indexes `p_problem` by loop index instead of planet and deletes 0 elements, so the timer==-1 cleanup never removes anything. The only caller (scr_enemy_ai_d.gml:87) also runs in obj_star scope where this method no longer exists, so the call errors. Rewrite the loop to delete from `system.p_problem[planet]` at index `i` (count 1) and route the caller through `get_planet_data(i).problem_count_down(...)`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| /// @self Asset.GMObject.obj_popup | ||
| function init_mission_inquisition_tomb_world() { | ||
| mission_star = find_star_by_name(pop_data.system); | ||
| var _p_data = mission_star.get_planet_data(pop_data.planet); |
There was a problem hiding this comment.
P1: This dereferences mission_star before the missing-star guard, so an invalid or deleted target crashes mission acceptance instead of closing the popup. Move get_planet_data after the guard.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_inquisition_mission/scr_inquisition_mission.gml, line 227:
<comment>This dereferences `mission_star` before the missing-star guard, so an invalid or deleted target crashes mission acceptance instead of closing the popup. Move `get_planet_data` after the guard.</comment>
<file context>
@@ -226,11 +224,12 @@ function mission_inquisition_tomb_world(tomb_worlds) {
/// @self Asset.GMObject.obj_popup
function init_mission_inquisition_tomb_world() {
mission_star = find_star_by_name(pop_data.system);
+ var _p_data = mission_star.get_planet_data(pop_data.planet);
if (mission_star == noone) {
popup_default_close();
</file context>
| var _p_data = mission_star.get_planet_data(pop_data.planet); | |
| var _p_data = mission_star == noone ? noone : mission_star.get_planet_data(pop_data.planet); |
| for (var i = array_length(problems) -1; i >= 0; i--) { | ||
| var _problem = problems[i]; | ||
| _problem.basic_turn_end(); | ||
| if (problem.timer == -1){ |
There was a problem hiding this comment.
P1: problem_count_down is broken: problem.timer is an undefined variable (should be _problem.timer), array_delete(system.p_problem[i], i, 0) indexes p_problem by loop index instead of planet and deletes 0 elements, so the timer==-1 cleanup never removes anything. The only caller (scr_enemy_ai_d.gml:87) also runs in obj_star scope where this method no longer exists, so the call errors. Rewrite the loop to delete from system.p_problem[planet] at index i (count 1) and route the caller through get_planet_data(i).problem_count_down(...).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/scr_PlanetData/scr_PlanetData.gml, line 802:
<comment>`problem_count_down` is broken: `problem.timer` is an undefined variable (should be `_problem.timer`), `array_delete(system.p_problem[i], i, 0)` indexes `p_problem` by loop index instead of planet and deletes 0 elements, so the timer==-1 cleanup never removes anything. The only caller (scr_enemy_ai_d.gml:87) also runs in obj_star scope where this method no longer exists, so the call errors. Rewrite the loop to delete from `system.p_problem[planet]` at index `i` (count 1) and route the caller through `get_planet_data(i).problem_count_down(...)`.</comment>
<file context>
@@ -760,24 +758,53 @@ function PlanetData(_planet, _system) constructor {
+ for (var i = array_length(problems) -1; i >= 0; i--) {
+ var _problem = problems[i];
+ _problem.basic_turn_end();
+ if (problem.timer == -1){
+ array_delete(system.p_problem[i], i,0);
+ array_delete(problems, i,0);
</file context>
Moves the Demon World problem creation logic directly into `scr_inquisition_mission`, eliminating a redundant helper function. Enhances `MissionHelper` to use dynamic function calls for mission acceptance, provide context-sensitive dialogue for demanded missions, and ensure popups close correctly.
|
/review |
lyra-the-bot
left a comment
There was a problem hiding this comment.
Incremental re-review of 8340070..811cd0a. Good progress, verified in workspace: Problem __popup_choice simplification + __add_option/LOGGER.error fail-loud, popup add_option always-wrap fix (raw options now get get_label/on_click), replace_options arity fix, crusade else if chain, p_data.name() demon-world fix. Still REQUEST_CHANGES: this increment reintroduces a compile error (fuction) plus undefined-var/scope bugs in the new mech_mars fleet mission, a dispatcher miss on tomb popups, inverted role conversion, a scr_role_count regression, and wrong popup scope in protect-raiders memorial.
|
/review |
lyra-the-bot
left a comment
There was a problem hiding this comment.
Incremental re-review of 811cd0a..2fda240. Good progress, verified in workspace: fuction->function fix, __inquisition_tomb_mission_start prefix fix, memorial obj_popup.text scope fix, !is_string role conversion fix, scr_role_count company overload, shared __save/load_members in Problem/FleetProblem/PlanetProblem, on_split/on_merge dedup to base (inherits correctly).
Still REQUEST_CHANGES: this increment adds blocking compile/runtime bugs (array_legth x2, broken singles loop, undefined planet/system, dead arrival loop, bare star/planet). Fix inline blockers first.
|
/review |
lyra-the-bot
left a comment
There was a problem hiding this comment.
Incremental re-review of 2fda240..4f406e9. Good progress, verified in workspace: singles-loop array_length/_r_data fix, mech-mars arrival loop i >= 0, array_length(members) fix, mission_log_entry dedup to Problem base, fleet_problems_to_mission_log helper shared by both fleet types, debug_slate.draw arity fix. Still REQUEST_CHANGES: this increment adds blocking runtime bugs in the touched hunks (bare orbiting + string-call, missing mark colour, undefined log rows, over-broad instance activation). Fix inline blockers first.
|
/review |
lyra-the-bot
left a comment
There was a problem hiding this comment.
Incremental re-review of 4f406e9..65877ac. Good progress, verified in workspace: mark(colour="green") default, _orbiting.name() scope fix, valid_special_feature() widening to Problem, cs_meeting_battle10 removal (now covered by generic battle_enemy_data.cols in Alarm_0), chaos_lord_meeting key consolidation in mission table/enemy-ai/dialogue/drop-select.
Still REQUEST_CHANGES: this increment adds blocking compile/runtime bugs in the touched hunks (inverted guards, dispatcher misses, dropped popup options/text, undefined vars, missing comma, bare defeat). Fix inline blockers first.
Carry-overs NOT in this diff (still blocking, no inline slot, verified at HEAD): (1) obj_controller/Create_0.gml removed current_eventing = "" init, but obj_popup/Destroy_0.gml:2,5,75-77, obj_ncombat/Alarm_7.gml:342,389, PlanetProblem:2081,2094,2159 still read/write obj_controller.current_eventing - first popup destroy before any write throws undefined; restore the init. (2) obj_popup/Destroy_0.gml:12-13 still has_problem_star("meeting") x2 (should be chaos_lord_meeting), :19 sets meeting_planet = meeting in the trap branch (should be trap value), :46 still remove_star_problem("meeting"). (3) obj_ncombat/Alarm_0.gml:136 still move_data_to_curret_scope typo (missing n) - any battle_enemy_data battle (incl. new chaos trap) throws on undefined function. (4) scr_dialogue.gml:380 duplicated has_problem_star("chaos_lord_meeting") > 0 && ... double call - second operand repeats the first; keep one.
| @@ -1,5 +1,5 @@ | |||
| //so this only runs if there aren't these types of instances | |||
| if (!instance_exists(obj_saveload) && !instance_exists(obj_popup) && !instance_exists(obj_ncombat) && !instance_exists(obj_fleet)) { | |||
| if (instance_exists(obj_saveload) && !instance_exists(obj_popup) && !instance_exists(obj_ncombat) && !instance_exists(obj_fleet)) { | |||
There was a problem hiding this comment.
Inverted guard: now requires obj_saveload to exist, so this whole block (ground/space combat clicks) never runs during normal play. Comment above says "only runs if there aren't these types of instances". Restore !instance_exists(obj_saveload).
| /// @description handles in game area and menu changes | ||
| /// @returns {bool} | ||
| function scr_change_menu(wanted_menu, specific_area_function = undefined) { | ||
| if (instance_exists(obj_n_combat)){ |
There was a problem hiding this comment.
obj_n_combat does not exist (object is obj_ncombat), so this never fires; logic is also inverted vs intent (was unconditional instance_activate_all(), which breaks deactivated-star combat state). Should be if (!instance_exists(obj_ncombat)) instance_activate_all();.
| if (!_master_present && (_force_size > 21)) { | ||
| _popup_text += "The meeting was supposed to be with the Chaos Lord, and yourself, but you are not planet-side. Land on the planet with up to two squads and the meeting will proceed." | ||
| } | ||
| scr_popup("Chaos Meeting", _popup_text, "chaos_messenger", _pop_data); |
There was a problem hiding this comment.
_options built above is never attached to _pop_data, so scr_popup queues a Chaos Meeting popup with zero buttons (softlock; scr_popup only wires pop_data.options). Add _pop_data.options = _options; before this call (cf. tomb pattern at :1513).
|
|
||
| static __chaos_lord_meeting_attend_meeting = function(){ | ||
| var _text = $"{global.chapter_name} signal your readiness to the heretic. Nearly twenty minutes of following the man passes before {global.chapter_name} all enter an ordinary-looking structure. Down, within the basement, {global.chapter_name} then pass into the entrance of a tunnel. As the trek downward continues more and more heretics appear- cultists, renegades that appear to be from the local garrison, and occasionally even the fallen of your kind. Overall the heretics seem well supplied and equip. This observation is interrupted as your group enters into a larger chamber, revealing a network of tunnels and what appears to be ancient catacombs. Bones of the ancient dead, the forgotten, litter the walls and floor. And the chamber seems to open up wider, and wider, until {global.chapter_name} find yourself within a hall. Within this hall, waiting for {global.chapter_name}" | ||
| if (!data.trap) { |
There was a problem hiding this comment.
data.trap is never set: creation site passes {is_trap: ...} (scr_chaos_alliance_test:74). Trap path is dead until one side is renamed (e.g. read data.is_trap).
| if (!data.trap) { | ||
| obj_controller.complex_event = true; | ||
| obj_controller.current_eventing = "chaos_meeting_1"; | ||
| obj_popup.text = $" are several dozen Chaos Terminators, a Greater Daemon of Tzeentch and Slaanesh, and Chaos Lord {obj_controller.faction_leader[eFACTION.CHAOS]}."; |
There was a problem hiding this comment.
var _text (:2078) is dropped; popup only gets the trailing fragment starting with " are several dozen...". Should be obj_popup.text = _text + $" are several...". Same defect in trap branch below.
| for (var i = 1; i <= 4;) { | ||
| for (var r = 1; r <= 4; r++) { | ||
| if ((p_problem[i][r] == "meeting") || (p_problem[i][r] == "meeting_trap")) { | ||
| if (p_problem[i][r].p_id == "chaos_lord_meeting") { |
There was a problem hiding this comment.
p_problem[i][r].p_id throws when the slot holds a non-struct (empty ""/0). Guard with is_struct(p_problem[i][r]) && p_problem[i][r].p_id == "chaos_lord_meeting".
//All missions that run on a fleet need their own FleetProblem instance
//missions for the most part can only run from predefined points in the code
// to run from each of these points each mission must have a registered function these functions are then called implicitly
//each function follows the format
__<p_id>_<entry_flag>/* the current entry points for the code are
"per_turn" - runs every end turn to check for certain conditions often contains reactions to actions player has done last turn
- runs via basic_turn_end
other than these entry points and any other later defined positions mission specific code should not run outside of the FleetProblem container
to set an entry point up simply register the function as a static e.g
if i create static __example_per_turn once an "example" problem is registered on a fleet the per turn check will run
no other code is required
other info
- the owning fleet must have a
problems = []array in its create event- entry points that receive extra information read it from fleet_event_data, which only exists for the duration of the call
- set a mission for deletion by setting delete_mission = true; this will delete the mission after the current function
has finished executing
- functions prefixed with
__are only accessible from within the FleetProblem's internal scope*/
//ANYTIME an exception is caught on a entry point for a mission the mission is prematurely deleted this ensures the player is not unfairly penalised for errors
Summary by cubic
Refactors planet missions from parallel arrays (
p_problem,p_timer,p_problem_other_data) into OOP problem objects, so active missions now persist across saves. Planet, system, and fleet missions inherit from a sharedProblemconstructor that standardizes lifecycle state, data access, and auto-deletion.per_turn,resolve,on_arrival,on_load,on_unload,on_split,on_merge,on_purge,mission_log_entry, etc.); an exception on any hook auto-deletes the mission. The mission log is now rebuilt from these per-problem hooks.system.problemswith turn-end countdowns; fleet missions areFleetProblemobjects (PlayerFleetProblem/AiFleetProblem) inobj_p_fleet/obj_en_fleet.alarm[1]intoplayer_fleet_end_turn(); arrival runs througharrived_at_star()backed byfleet_is_orbiting()andget_orbiting_player_fleet()helpers, which also fixes craft world discovery on arrival.fleet_engagedandfleet_move_optionsare replaced by abeyond_engagementflag; enemy navy fleets track inbound player fleets withnavy_enemy_fleet_enroute.special_featurestruct, enemy forces come frombattle_enemy_datacreated throughadd_enemiesonobj_enunit, and casualties feed mission hooks throughregister_kills_to_missions.PlanetProblem; adds a Tyranid "Hive Fleet to Cult" mission; removesobj_temp7and legacy helper scripts; the debug mission-timer overlay is replaced by a system debug slate.squad.membersaccess goes throughsquad.get_members(), which prunes stale marine entries;PLANET_ARRAY_SIZEreplaces the hard-coded planet count;fleet_full_ship_arraybecomes afull_ship_array()fleet method.Migration
mech_tomb1/mech_tomb2→mech_tomb,meeting/meeting_trap→chaos_lord_meeting,fallen→hunt_fallen,spyrer→inquisition_spyrer,recon→inquisition_recon,tyranid_org→inquisition_tyranid_org,necron/inquisition_tomb→inquisition_necron;mech_bionicsis removed.add_new_problemwithPlanetData.add_problem; missions signal completion by setting adelete_missionflag instead of callingremove_planet_problem.fallencheat command and legacy variables likebattle_mission,battle_data, andcaptured_gauntare gone.Written for commit 85ff92a. Summary will update on new commits.